Remove all uses of sprintf (#958)
authorRobert Lipe <robertlipe@users.noreply.github.com>
Thu, 22 Dec 2022 23:12:31 +0000 (17:12 -0600)
committerGitHub <noreply@github.com>
Thu, 22 Dec 2022 23:12:31 +0000 (17:12 -0600)
Deprecate sprintf in favor of snprintf or QString::asprintf... This buys time until we have {fmt}

* remove unused bgr in tpo.cc
* more pruning of C-style constructs
* Remove unused lap code in Garmin
Unrelated, but required to build on Mac
* Apply Mac runner updates per active topics from runner images bugreports.
It's my belief there are transient problems in the GitHub-provided images (linked in this CL) but this bulletproofs our installation, so I'm rolling with it.

.github/workflows/macos.yml
duplicate.cc
garmin.cc
holux.cc
jeeps/gpsapp.cc
jeeps/gpscom.cc
jeeps/gpsprot.cc
mkshort.cc
mtk_logger.cc
tpo.cc
wbt-200.cc

index bedb27a5c4634572c50ed7b75dcb525ef0a831f8..7de263fee833f112ca523f733bcd90207dc0cb7f 100644 (file)
@@ -61,7 +61,17 @@ jobs:
     - name: Brew install
       if: matrix.GENERATOR == 'Ninja'
       run: |
+        # homebrew fails to update python from 3.9 to 3.10 due
+        # to another unlinking failure. See hack at brew upgrade
+        rm -f /usr/local/bin/2to3 /usr/local/bin/idle3 \
+          /usr/local/bin/idle3 /usr/local/bin/pydoc3 \
+          /usr/local/bin/python3 /usr/local/bin/python3-config
         brew update
+        # From https://github.com/actions/runner-images/issues/6817#issuecomment-1363382175
+        # a topic that's flaming right now, though this is a recurring
+        # problem as described in
+        # https://github.com/actions/runner-images/issues/4020
+        brew upgrade || true
         brew install ninja
         brew install docbook docbook-xsl fop gnu-sed
         brew install jing-trang
index b18296324cbcf270343218fc34c76ee43ddac715..98f5d92d27d6cdc8f6ca7481c2e97e0c2386ad12 100644 (file)
@@ -20,7 +20,7 @@
  */
 
 #include <algorithm>            // for stable_sort
-#include <cstdio>               // for sprintf
+#include <cstdio>               // for snprintf
 #include <cstring>              // for memset, strncpy
 
 #include <QDateTime>            // for QDateTime
@@ -142,15 +142,14 @@ void DuplicateFilter::process()
     }
 
     if (lcopt) {
-      /* let sprintf take care of rounding */
-      sprintf(dupe.lat, "%11.4f", waypointp->latitude);
-      sprintf(dupe.lon, "%11.4f", waypointp->longitude);
       /* The degrees2ddmm stuff is a feeble attempt to
        * get everything rounded the same way in a precision
        * that's "close enough" for determining duplicates.
        */
-      sprintf(dupe.lat, "%11.3f", degrees2ddmm(waypointp->latitude));
-      sprintf(dupe.lon, "%11.3f", degrees2ddmm(waypointp->longitude));
+      snprintf(dupe.lat, sizeof(dupe.lat), "%11.3f",
+               degrees2ddmm(waypointp->latitude));
+      snprintf(dupe.lon, sizeof(dupe.lon), "%11.3f",
+               degrees2ddmm(waypointp->longitude));
 
     }
 
index d846475795388743090371c11eaaeee02e6fefd8..a7c0c43a24a1d41a7854c5c01402ff43f021997c 100644 (file)
--- a/garmin.cc
+++ b/garmin.cc
@@ -23,7 +23,7 @@
 #include <climits>              // for INT_MAX
 #include <cmath>                // for atan2, floor, sqrt
 #include <csetjmp>              // for setjmp
-#include <cstdio>               // for fprintf, fflush, snprintf, sprintf
+#include <cstdio>               // for fprintf, fflush, snprintf, snprintf
 #include <cstdlib>              // for strtol
 #include <cstring>              // for memcpy, strlen, strncpy, strchr
 #include <ctime>                // for time_t
@@ -615,97 +615,6 @@ route_read()
 
 }
 
-#if 0
-static
-void
-lap_read_as_track(void)
-{
-  int32 ntracks;
-  GPS_PLap* array;
-  route_head* trk_head = NULL;
-  int trk_num = 0;
-  int index;
-  int i;
-  char tbuf[128];
-
-  ntracks = GPS_Command_Get_Lap(portname, &array, waypt_read_cb);
-  if (ntracks <= 0) {
-    return;
-  }
-  for (i = 0; i < ntracks; i++) {
-    Waypoint* wpt;
-    if (array[i]->index == -1) {
-      index=i;
-    } else {
-      index=array[i]->index;
-      index=i;
-    }
-
-    if ((trk_head == NULL) || (i == 0) ||
-        /* D906 - last track:index is the track index */
-        (array[i]->index == -1 && array[i]->track_index != 255) ||
-        /* D10xx - no real separator, use begin/end time to guess */
-        (abs(array[i-1]->start_time + array[i]->total_time/100-array[i]->start_time) > 2)
-       ) {
-      static struct tm* stmp;
-      stmp = gmtime(&array[i]->start_time);
-      trk_head = new route_head;
-      /*For D906, we would like to use the track_index in the last packet instead...*/
-      trk_head->rte_num = ++trk_num;
-      strftime(tbuf, 32, "%Y-%m-%dT%H:%M:%SZ", stmp);
-      trk_head->rte_name = tbuf;
-      track_add_head(trk_head);
-
-      wpt = new Waypoint;
-
-      wpt->longitude = array[i]->begin_lon;
-      wpt->latitude = array[i]->begin_lat;
-      wpt->heartrate = array[i]->avg_heart_rate;
-      wpt->cadence = array[i]->avg_cadence;
-      wpt->speed = array[i]->max_speed;
-      wpt->creation_time = array[i]->start_time;
-      wpt->microseconds = 0;
-
-      sprintf(tbuf, "#%d-0", index);
-      wpt->shortname = tbuf;
-      sprintf(tbuf, "D:%f Cal:%d MS:%f AH:%d MH:%d AC:%d I:%d T:%d",
-              array[i]->total_distance, array[i]->calories, array[i]->max_speed, array[i]->avg_heart_rate,
-              array[i]->max_heart_rate, array[i]->avg_cadence, array[i]->intensity, array[i]->trigger_method);
-      wpt->description = tbuf;
-      track_add_wpt(trk_head, wpt);
-    }
-    /*Allow even if no correct location, no skip if invalid */
-    /*         if (array[i]->no_latlon) {
-    *                  continue;
-    *          }
-    */
-    wpt = new Waypoint;
-
-    wpt->longitude = array[i]->end_lon;
-    wpt->latitude = array[i]->end_lat;
-    wpt->heartrate = array[i]->avg_heart_rate;
-    wpt->cadence = array[i]->avg_cadence;
-    wpt->speed = array[i]->max_speed;
-    wpt->creation_time = array[i]->start_time + array[i]->total_time/100;
-    wpt->microseconds = 10000*(array[i]->total_time % 100);
-    /*Add fields with no mapping in the description */
-    sprintf(tbuf, "#%d", index);
-    wpt->shortname = tbuf;
-    sprintf(tbuf, "D:%f Cal:%d MS:%f AH:%d MH:%d AC:%d I:%d T:%d (%f,%f)",
-            array[i]->total_distance, array[i]->calories, array[i]->max_speed, array[i]->avg_heart_rate,
-            array[i]->max_heart_rate, array[i]->avg_cadence, array[i]->intensity, array[i]->trigger_method,
-            array[i]->begin_lon, array[i]->begin_lat);
-    wpt->description = tbuf;
-
-    track_add_wpt(trk_head, wpt);
-  }
-  while (ntracks) {
-    GPS_Lap_Del(&array[--ntracks]);
-  }
-  xfree(array);
-}
-#endif
-
 /*
  * Rather than propagate Garmin-specific data types outside of the Garmin
  * code, we convert the PVT (position/velocity/time) data from the receiver
@@ -1131,7 +1040,7 @@ track_hdr_pr(const route_head* trk_head)
     strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(trk_head->rte_name), sizeof((*cur_tx_tracklist_entry)->trk_ident) - 1);
     (*cur_tx_tracklist_entry)->trk_ident[sizeof((*cur_tx_tracklist_entry)->trk_ident)-1] = 0;
   } else {
-    sprintf((*cur_tx_tracklist_entry)->trk_ident, "TRACK%02d", my_track_count);
+    snprintf((*cur_tx_tracklist_entry)->trk_ident, sizeof((*cur_tx_tracklist_entry)->trk_ident), "TRACK%02d", my_track_count);
   }
   cur_tx_tracklist_entry++;
   my_track_count++;
index 4d3c4ab81abc5dcc5a302a52e4444fef2dcbc7ad..b72b5c0bf2a75796102e074a7f952608c93506e9 100644 (file)
--- a/holux.cc
+++ b/holux.cc
@@ -26,7 +26,7 @@ History:
 /* This module is for the holux (gm-100) .wpo format */
 
 #include <cstring>                 // for strncpy, memset, strcpy, strlen
-#include <cstdio>                  // for sprintf
+#include <cstdio>                  // for snprintf
 #include <ctime>                   // for gmtime, mktime, time_t, tm
 
 #include <QDate>                   // for QDate
@@ -113,10 +113,10 @@ static void data_read()
     WPT* pWptHxTmp = (WPT*)&HxWpt[OFFS_WPT + (sizeof(WPT) * iWptIndex)];
 
     wpt_tmp->altitude = 0;
-    strncpy(name,pWptHxTmp->name,sizeof(pWptHxTmp->name));
+    strncpy(name,pWptHxTmp->name,sizeof(name));
     name[sizeof(pWptHxTmp->name)]=0;
 
-    strncpy(desc,pWptHxTmp->comment,sizeof(pWptHxTmp->comment));
+    strncpy(desc,pWptHxTmp->comment,sizeof(desc));
     desc[sizeof(pWptHxTmp->comment)]=0;
 
     wpt_tmp->shortname = name;
@@ -210,7 +210,7 @@ static void holux_disp(const Waypoint* wpt)
   if (wpt->shortname != nullptr) {
     strncpy(pWptHxTmp->name, mknshort(CSTRc(wpt->shortname),sizeof(pWptHxTmp->name)),sizeof(pWptHxTmp->name));
   } else {
-    sprintf(pWptHxTmp->name,"W%d",sIndex);
+    snprintf(pWptHxTmp->name,sizeof(pWptHxTmp->name), "W%d",sIndex);
   }
 
   memset(pWptHxTmp->comment,0x20,sizeof(pWptHxTmp->comment));
index fcfbead3936836353a399527f95e10140c058466..1d28ed0efd23d6d83810cff1f52e2609bf40ac04 100644 (file)
@@ -4486,7 +4486,7 @@ void GPS_D311_Get(GPS_PTrack* trk, UC* s)
 
   /* Forerunner */
   identifier = GPS_Util_Get_Short(s);
-  sprintf((*trk)->trk_ident, "%d", identifier);
+  snprintf((*trk)->trk_ident, sizeof((*trk)->trk_ident), "%d", identifier);
 
   return;
 }
index e209ece092bbc5560f44422877c72a379dbbe189..f15c8f514ee82cf1095e18ba46b2cc159c7306db 100644 (file)
@@ -1235,7 +1235,8 @@ int32 GPS_Command_Send_Track_As_Course(const char* port, GPS_PTrack* trk, int32
     if (trk[i]->ishdr) {
       /* Index of new track, must match the track index in associated course */
       memset(ctk[n_ctk]->trk_ident, 0, sizeof(ctk[n_ctk]->trk_ident));
-      sprintf(ctk[n_ctk]->trk_ident, "%u", crs[new_crs]->track_index);
+      snprintf(ctk[n_ctk]->trk_ident, sizeof(ctk[n_ctk]->trk_ident),
+               "%u", crs[new_crs]->track_index);
       new_crs++;
     }
     n_ctk++;
index eaab19df066e780fc4a3069260b8e8e0f568b6d0..91332bfee5fd8a6d8db385b5746508e15a23fdc2 100644 (file)
@@ -372,7 +372,7 @@ int32 GPS_Protocol_Table_Set(US id)
   }
 
 
-  (void)sprintf(s,"INIT: No table entry for ID %d\n",id);
+  (void) snprintf(s, sizeof(s), "INIT: No table entry for ID %d\n", id);
   GPS_Error(s);
 
   return GPS_UNSUPPORTED;
@@ -394,7 +394,8 @@ void GPS_Protocol_Error(US tag, US data)
 {
   char s[GPS_ARB_LEN];
 
-  (void) sprintf(s,"PROTOCOL ERROR: Unknown tag/data [%c/%d]\n",tag,data);
+  (void) snprintf(s, sizeof(s),
+                 "PROTOCOL ERROR: Unknown tag/data [%c/%d]\n", tag, data);
   GPS_Error(s);
 
   if (gps_n_tag_unknown < GPS_TAGUNK) {
index 3e42244d8c584194a0566316d59154a594db2eef..34499a1d56fb41834720b1a64eef1affc88eff22 100644 (file)
@@ -20,7 +20,7 @@
  */
 
 #include <cctype>      // for isspace, toupper, isdigit
-#include <cstdio>      // for sprintf, size_t
+#include <cstdio>      // for snprintf, size_t
 #include <cstring>     // for strlen, memmove, strchr, strcpy, strncmp, strcat, strncpy
 
 #include <QByteArray>  // for QByteArray
@@ -143,7 +143,7 @@ mkshort_add_to_list(mkshort_handle_imp* h, char* name)
 
     s->conflictctr++;
 
-    int dl = sprintf(tbuf, ".%d", s->conflictctr);
+    int dl = snprintf(tbuf, sizeof(tbuf), ".%d", s->conflictctr);
 
     if (l + dl < h->target_len) {
       name = (char*) xrealloc(name, l + dl + 1);
index 7cc640902b23ce381cd040c09f6565b7e6dcfc4c..fdb014782c8cbd83909f0a3fbba59908e66bf2c0 100644 (file)
@@ -801,16 +801,17 @@ static int add_trackpoint(int idx, unsigned long bmask, struct data_item* itm)
   auto* trk = new Waypoint;
 
   if (global_opts.masked_objective& TRKDATAMASK && (trk_head == nullptr || (mtk_info.track_event & MTK_EVT_START))) {
-    char spds[50];
     trk_head = new route_head;
     trk_head->rte_name = QStringLiteral("track-%1").arg(1 + track_count());
 
-    spds[0] = '\0';
+    QString spds;
     if (mtk_info.speed > 0) {
-      sprintf(spds, " when moving above %.0f km/h", mtk_info.speed/10.);
+      spds = QString::asprintf(" when moving above %.0f km/h", mtk_info.speed/10.);
     }
-    trk_head->rte_desc = QString::asprintf("Log every %.0f sec, %.0f m%s"
-                                           , mtk_info.period/10., mtk_info.distance/10., spds);
+    trk_head->rte_desc = QString::asprintf("Log every %.0f sec, %.0f m",
+                                           mtk_info.period/10.,
+                                           mtk_info.distance/10.);
+    trk_head->rte_desc += spds;
     track_add_head(trk_head);
   }
 
@@ -979,12 +980,7 @@ static void mtk_csv_deinit()
 /* Output a single data line in MTK application compatible format - i.e ignore any locale settings... */
 static int csv_line(gbfile* csvFile, int idx, unsigned long bmask, struct data_item* itm)
 {
-  char ts_str[30];
   const char* fix_str = "";
-
-  struct tm* ts_tm = gmtime(&(itm->timestamp));
-  strftime(ts_str, sizeof(ts_str)-1, "%Y/%m/%d,%H:%M:%S", ts_tm);
-
   if (bmask & (1U<<VALID)) {
     switch (itm->valid) {
     case 0x0001:
@@ -1028,7 +1024,12 @@ static int csv_line(gbfile* csvFile, int idx, unsigned long bmask, struct data_i
               , (itm->rcr&0x0004)?"D":"", (itm->rcr&0x0008)?"B":"");
 
   if (bmask & (1U<<UTC)) {
-    gbfprintf(csvFile, "%s.%.3d,", ts_str, (bmask & (1U<<MILLISECOND))?itm->timestamp_ms:0);
+    QDateTime dt = QDateTime::fromSecsSinceEpoch(itm->timestamp, Qt::UTC);
+    dt = dt.addMSecs(itm->timestamp_ms);
+
+    QString timestamp = dt.toUTC().toString("yyyy/MM/dd,hh:mm:ss.zzz");;
+    gbfputs(timestamp, csvFile);
+    gbfputc(',', csvFile);
   }
 
   if (bmask & (1U<<VALID)) {
@@ -1072,25 +1073,24 @@ static int csv_line(gbfile* csvFile, int idx, unsigned long bmask, struct data_i
   }
 
   if (bmask & (1U<<SID)) {
-    int do_sc = 0;
-    char sstr[40];
-    for (int l=0; l<itm->sat_count; l++) {
-      int slen = 0;
-      slen += sprintf(&sstr[slen], "%s%.2d"
-                      , itm->sat_data[l].used?"#":""
-                      , itm->sat_data[l].id);
+    for (int l = 0; l < itm->sat_count; l++) {
+      QString s = QString::asprintf("%s%.2d",
+                                    itm->sat_data[l].used ? "#" : "",
+                                    itm->sat_data[l].id);
       if (bmask & (1U<<ELEVATION)) {
-        slen += sprintf(&sstr[slen], "-%.2d", itm->sat_data[l].elevation);
+        s += QString::asprintf("-%.2d", itm->sat_data[l].elevation);
       }
       if (bmask & (1U<<AZIMUTH)) {
-        slen += sprintf(&sstr[slen], "-%.2d", itm->sat_data[l].azimut);
+        s += QString::asprintf("-%.2d", itm->sat_data[l].azimut);
       }
       if (bmask & (1U<<SNR)) {
-        slen += sprintf(&sstr[slen], "-%.2d", itm->sat_data[l].snr);
+        s += QString::asprintf("-%.2d", itm->sat_data[l].snr);
       }
 
-      gbfprintf(csvFile, "%s%s", do_sc?";":"", sstr);
-      do_sc = 1;
+      if (l) {
+        gbfputc(';', csvFile);
+      }
+      gbfputs(s, csvFile);
     }
     gbfprintf(csvFile, ",");
   }
diff --git a/tpo.cc b/tpo.cc
index 896bda59be8b4cd267137cc56d2811e63536a779..f37fdd44aa791a71600eff14cdc3c63ef64a5b38 100644 (file)
--- a/tpo.cc
+++ b/tpo.cc
@@ -71,7 +71,7 @@
 
 #include <cassert>                     // for assert
 #include <cmath>                       // for cos, sqrt
-#include <cstdio>                      // for printf, sprintf, SEEK_CUR, SEEK_SET
+#include <cstdio>                      // for printf, SEEK_CUR, SEEK_SET
 #include <cstdint>
 #include <cstring>                     // for strncmp, strlen, memset
 #include <vector>                      // for vector
@@ -636,7 +636,6 @@ static void tpo_process_tracks()
     }
     int lat = 0;
     int lon = 0;
-    char rgb[7],bgr[7];
 
     // Allocate the track struct
     auto* track_temp = new route_head;
@@ -670,9 +669,13 @@ static void tpo_process_tracks()
     track_temp->rte_name = track_name;
 
     // RGB line_color expressed for html=rrggbb and kml=bbggrr - not assigned before 2012
-    sprintf(rgb,"%02x%02x%02x",styles[track_style].color[0],styles[track_style].color[1],styles[track_style].color[2]);
-    sprintf(bgr,"%02x%02x%02x",styles[track_style].color[2],styles[track_style].color[1],styles[track_style].color[0]);
-    int bbggrr = styles[track_style].color[2] << 16 | styles[track_style].color[1] << 8 | styles[track_style].color[0];
+    auto rgb = QString::asprintf("%02x%02x%02x",
+                                 styles[track_style].color[0],
+                                 styles[track_style].color[1],
+                                 styles[track_style].color[2]);
+    int bbggrr = styles[track_style].color[2] << 16 |
+                 styles[track_style].color[1] << 8 |
+                 styles[track_style].color[0];
     track_temp->line_color.bbggrr = bbggrr;
 
     // track texture (dashed=1, solid=0) mapped into opacity - not assigned before 2012
@@ -687,7 +690,10 @@ static void tpo_process_tracks()
 
     if constexpr(debug) {
       printf("Track Name: %s, ?Type?: %u, Style Name: %s, Width: %d, Dashed: %d, Color: #%s\n",
-             qPrintable(track_name), line_type, qPrintable(styles[track_style].name), styles[track_style].wide, styles[track_style].dash,rgb);
+             qPrintable(track_name), line_type,
+             qPrintable(styles[track_style].name),
+             styles[track_style].wide, styles[track_style].dash,
+             qPrintable(rgb));
     }
 
     // Track description
index 7efd06ed8b53612205b47d1d1cc3d4b22d580289..c90d312f661e8005a88d5d20ef9ce97d3ae12ff0 100644 (file)
@@ -21,7 +21,7 @@
 #include <array>                   // for array
 #include <cctype>                  // for isprint
 #include <cstdarg>                 // for va_end, va_list, va_start
-#include <cstdio>                  // for size_t, sprintf, fread, fclose, feof
+#include <cstdio>                  // for size_t, snprintf, fread, fclose, feof
 #include <cstdint>                 // for uint32_t, int32_t, int16_t, uint16_t
 #include <cstdlib>                 // for strtod, strtoul
 #include <cstring>                 // for strlen, memcmp, memcpy
@@ -551,16 +551,13 @@ static int check_date(uint32_t tim)
 
 static Waypoint* make_point(double lat, double lon, double alt, time_t tim, const char* fmt, int index)
 {
-  char     wp_name[20];
   auto* wpt = new Waypoint;
 
-  sprintf(wp_name, fmt, index);
-
   wpt->latitude       = lat;
   wpt->longitude      = lon;
   wpt->altitude       = alt;
   wpt->SetCreationTime(tim);
-  wpt->shortname      = wp_name;
+  wpt->shortname      = QString::asprintf(fmt, index);
 
   return wpt;
 }
@@ -777,12 +774,12 @@ static void wbt200_data_read()
     int f;
     db(1, "Erasing data\n");
     for (f = 27; f <= 31; f++) {
-      sprintf(line_buf, "$PFST,REMOVEFILE,%d", f);
+      snprintf(line_buf, sizeof(line_buf), "$PFST,REMOVEFILE,%d", f);
       do_cmd(line_buf, "$PFST,REMOVEFILE", BUFSPEC(line_buf));
     }
     db(1, "Reclaiming free space\n");
     for (f = 0; f <= 3; f++) {
-      sprintf(line_buf, "$PFST,FFSRECLAIM,%d", f);
+      snprintf(line_buf, sizeof(line_buf), "$PFST,FFSRECLAIM,%d", f);
       do_cmd(line_buf, "$PFST,FFSRECLAIM", BUFSPEC(line_buf));
     }
   }
@@ -882,7 +879,7 @@ static int wbt201_read_chunk(struct read_state* st, unsigned pos, unsigned limit
   buf_empty(&st->data);
 
   rd_drain();
-  sprintf(cmd_buf, "@AL,5,3,%u", pos);
+  snprintf(cmd_buf, sizeof(cmd_buf), "@AL,5,3,%u", pos);
   wr_cmdl(cmd_buf);